-
Notifications
You must be signed in to change notification settings - Fork 1.1k
utils/deps: always show missing deps #4876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
Thanks for your contribution! |
|
Please merge the commits to the develop branch. |
031c922 to
1c5df61
Compare
Done. |
Bobholamovic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that is_extra_available is sometimes used only to check whether the extra is available in order to branch the logic, logging an error-level message here does not seem appropriate.
I suggest adding a parameter to is_extra_available to control whether an error-level log should be emitted. This flag can then be enabled in assertion-style functions such as require_extra, so that users can still receive helpful diagnostic information when necessary.
When first trying out paddlex, it can be hard to figure out how to increase the logging verbosity. If there are missing dependencies and this raises an error, seeing the exact list is helpful.
1c5df61 to
78fdb9b
Compare
|
OK I see this makes sense, I wasn't aware that |
When trying to use PaddleX on NixOS, which doesn't allow imperative installation of Python packages with
pip, I found this change to be useful, and thought that other users might benefit from it. Original commit message: